Skip to content

Minimise allocations for ModifierSum#739

Open
wvpm wants to merge 1 commit into
masterfrom
minimise_allocations_modifiersum
Open

Minimise allocations for ModifierSum#739
wvpm wants to merge 1 commit into
masterfrom
minimise_allocations_modifiersum

Conversation

@wvpm
Copy link
Copy Markdown
Contributor

@wvpm wvpm commented May 17, 2026

CountryInstance::contribute_province_modifier_sum is called for each province (expect 10s-100s per country). Every call to ModifierSum::add_modifier_sum called reserve_more, which is defined as:

    constexpr void reserve_more(reservable auto& t, size_t size) {
        t.reserve(t.size() + size);
    }

This resulted in repeated allocations, possibly reallocating the country's modifiersum for each controlled province.
Instead we now first sum the extra capacity for all provinces and then reserve once.

@wvpm wvpm added the enhancement New feature or request label May 17, 2026
@wvpm wvpm requested a review from a team as a code owner May 17, 2026 13:12
@wvpm wvpm enabled auto-merge May 17, 2026 13:14
@wvpm wvpm force-pushed the minimise_allocations_modifiersum branch from 2351277 to 99490eb Compare May 17, 2026 13:25
@schombert
Copy link
Copy Markdown

As a general note, calling reserve explicitly is typically only an optimization if you can find a one-time upper bound on the possible growth and thereby call reserve once instead of allowing the vector to grow naturally. However, if you call reserve repeatedly you run the risk of performing much worse than the vector's normal exponential growth. The growth factor of the vector is designed so that adding elements is amortized O(1). Repeated calls to reserve that grow the vector only by some amount that we can place an upper bound on (as presumably happens here, since the total number of possible modifiers is bounded) makes the entire process (cycles of reserve + element additions) amortized O(n). In your particular case, a little back of the napkin math will show that just allocating space for every country to have every possible modifier times the number of land provinces is within the memory budget, so you can use that as the value for a single reservation and be very, very unlikely to ever need to grow the vectors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request topic:codestyle

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants